Skip to content

Conversation

@Xazax-hun
Copy link
Collaborator

This patch makes the position -1 interpreted as the position for 'this'. Adds some basic infrastructure and support for lifetimebound attribute.

@Xazax-hun Xazax-hun force-pushed the apinotes-lifetimebound-this branch from 77fa0b6 to 81b8fae Compare November 6, 2024 12:24
@Xazax-hun Xazax-hun marked this pull request as ready for review November 6, 2024 12:24
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2024

@llvm/pr-subscribers-clang

Author: Gábor Horváth (Xazax-hun)

Changes

This patch makes the position -1 interpreted as the position for 'this'. Adds some basic infrastructure and support for lifetimebound attribute.


Full diff: https://github.com/llvm/llvm-project/pull/115021.diff

10 Files Affected:

  • (modified) clang/include/clang/APINotes/Types.h (+18-7)
  • (modified) clang/lib/APINotes/APINotesFormat.h (+1-1)
  • (modified) clang/lib/APINotes/APINotesReader.cpp (+18)
  • (modified) clang/lib/APINotes/APINotesTypes.cpp (+8)
  • (modified) clang/lib/APINotes/APINotesWriter.cpp (+24-5)
  • (modified) clang/lib/APINotes/APINotesYAMLCompiler.cpp (+22-7)
  • (modified) clang/lib/Sema/SemaAPINotes.cpp (+16)
  • (modified) clang/test/APINotes/Inputs/Headers/Lifetimebound.apinotes (+4)
  • (modified) clang/test/APINotes/Inputs/Headers/Lifetimebound.h (-1)
  • (modified) clang/test/APINotes/lifetimebound.cpp (+3)
diff --git a/clang/include/clang/APINotes/Types.h b/clang/include/clang/APINotes/Types.h
index 6327b7d75486fc..6ad1c850701146 100644
--- a/clang/include/clang/APINotes/Types.h
+++ b/clang/include/clang/APINotes/Types.h
@@ -445,9 +445,7 @@ class ParamInfo : public VariableInfo {
         RawRetainCountConvention() {}
 
   std::optional<bool> isNoEscape() const {
-    if (!NoEscapeSpecified)
-      return std::nullopt;
-    return NoEscape;
+    return NoEscapeSpecified ? std::optional<bool>(NoEscape) : std::nullopt;
   }
   void setNoEscape(std::optional<bool> Value) {
     NoEscapeSpecified = Value.has_value();
@@ -455,9 +453,8 @@ class ParamInfo : public VariableInfo {
   }
 
   std::optional<bool> isLifetimebound() const {
-    if (!LifetimeboundSpecified)
-      return std::nullopt;
-    return Lifetimebound;
+    return LifetimeboundSpecified ? std::optional<bool>(Lifetimebound)
+                                  : std::nullopt;
   }
   void setLifetimebound(std::optional<bool> Value) {
     LifetimeboundSpecified = Value.has_value();
@@ -643,6 +640,8 @@ class ObjCMethodInfo : public FunctionInfo {
   LLVM_PREFERRED_TYPE(bool)
   unsigned RequiredInit : 1;
 
+  std::optional<ParamInfo> Self;
+
   ObjCMethodInfo() : DesignatedInit(false), RequiredInit(false) {}
 
   friend bool operator==(const ObjCMethodInfo &, const ObjCMethodInfo &);
@@ -664,7 +663,7 @@ class ObjCMethodInfo : public FunctionInfo {
 inline bool operator==(const ObjCMethodInfo &LHS, const ObjCMethodInfo &RHS) {
   return static_cast<const FunctionInfo &>(LHS) == RHS &&
          LHS.DesignatedInit == RHS.DesignatedInit &&
-         LHS.RequiredInit == RHS.RequiredInit;
+         LHS.RequiredInit == RHS.RequiredInit && LHS.Self == RHS.Self;
 }
 
 inline bool operator!=(const ObjCMethodInfo &LHS, const ObjCMethodInfo &RHS) {
@@ -693,8 +692,20 @@ class FieldInfo : public VariableInfo {
 class CXXMethodInfo : public FunctionInfo {
 public:
   CXXMethodInfo() {}
+
+  std::optional<ParamInfo> This;
+
+  LLVM_DUMP_METHOD void dump(llvm::raw_ostream &OS);
 };
 
+inline bool operator==(const CXXMethodInfo &LHS, const CXXMethodInfo &RHS) {
+  return static_cast<const FunctionInfo &>(LHS) == RHS && LHS.This == RHS.This;
+}
+
+inline bool operator!=(const CXXMethodInfo &LHS, const CXXMethodInfo &RHS) {
+  return !(LHS == RHS);
+}
+
 /// Describes API notes data for an enumerator.
 class EnumConstantInfo : public CommonEntityInfo {
 public:
diff --git a/clang/lib/APINotes/APINotesFormat.h b/clang/lib/APINotes/APINotesFormat.h
index 014ee7e2e3d397..bdd379ca4ae956 100644
--- a/clang/lib/APINotes/APINotesFormat.h
+++ b/clang/lib/APINotes/APINotesFormat.h
@@ -24,7 +24,7 @@ const uint16_t VERSION_MAJOR = 0;
 /// API notes file minor version number.
 ///
 /// When the format changes IN ANY WAY, this number should be incremented.
-const uint16_t VERSION_MINOR = 31; // lifetimebound
+const uint16_t VERSION_MINOR = 32; // implicit parameter support (at position -1)
 
 const uint8_t kSwiftCopyable = 1;
 const uint8_t kSwiftNonCopyable = 2;
diff --git a/clang/lib/APINotes/APINotesReader.cpp b/clang/lib/APINotes/APINotesReader.cpp
index 1bde8482fce033..0f8fa1355bfb0a 100644
--- a/clang/lib/APINotes/APINotesReader.cpp
+++ b/clang/lib/APINotes/APINotesReader.cpp
@@ -14,6 +14,7 @@
 //===----------------------------------------------------------------------===//
 #include "clang/APINotes/APINotesReader.h"
 #include "APINotesFormat.h"
+#include "clang/APINotes/Types.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Bitstream/BitstreamReader.h"
@@ -396,12 +397,19 @@ class ObjCMethodTableInfo
                                         const uint8_t *&Data) {
     ObjCMethodInfo Info;
     uint8_t Payload = *Data++;
+    bool HasSelf = Payload & 0x01;
+    Payload >>= 1;
     Info.RequiredInit = Payload & 0x01;
     Payload >>= 1;
     Info.DesignatedInit = Payload & 0x01;
     Payload >>= 1;
+    assert(Payload == 0 && "Unable to fully decode 'Payload'.");
 
     ReadFunctionInfo(Data, Info);
+    if (HasSelf) {
+      Info.Self = ParamInfo{};
+      ReadParamInfo(Data, *Info.Self);
+    }
     return Info;
   }
 };
@@ -516,7 +524,17 @@ class CXXMethodTableInfo
   static CXXMethodInfo readUnversioned(internal_key_type Key,
                                        const uint8_t *&Data) {
     CXXMethodInfo Info;
+
+    uint8_t Payload = *Data++;
+    bool HasThis = Payload & 0x01;
+    Payload >>= 1;
+    assert(Payload == 0 && "Unable to fully decode 'Payload'.");
+
     ReadFunctionInfo(Data, Info);
+    if (HasThis) {
+      Info.This = ParamInfo{};
+      ReadParamInfo(Data, *Info.This);
+    }
     return Info;
   }
 };
diff --git a/clang/lib/APINotes/APINotesTypes.cpp b/clang/lib/APINotes/APINotesTypes.cpp
index be7dec3295b4ce..b92faa0acc7482 100644
--- a/clang/lib/APINotes/APINotesTypes.cpp
+++ b/clang/lib/APINotes/APINotesTypes.cpp
@@ -85,10 +85,18 @@ LLVM_DUMP_METHOD void FunctionInfo::dump(llvm::raw_ostream &OS) const {
 
 LLVM_DUMP_METHOD void ObjCMethodInfo::dump(llvm::raw_ostream &OS) {
   static_cast<FunctionInfo &>(*this).dump(OS);
+  if (Self)
+    Self->dump(OS);
   OS << (DesignatedInit ? "[DesignatedInit] " : "")
      << (RequiredInit ? "[RequiredInit] " : "") << '\n';
 }
 
+LLVM_DUMP_METHOD void CXXMethodInfo::dump(llvm::raw_ostream &OS) {
+  static_cast<FunctionInfo &>(*this).dump(OS);
+  if (This)
+    This->dump(OS);
+}
+
 LLVM_DUMP_METHOD void TagInfo::dump(llvm::raw_ostream &OS) {
   static_cast<CommonTypeInfo &>(*this).dump(OS);
   if (HasFlagEnum)
diff --git a/clang/lib/APINotes/APINotesWriter.cpp b/clang/lib/APINotes/APINotesWriter.cpp
index d81394edfde304..3b8b3c80f7c086 100644
--- a/clang/lib/APINotes/APINotesWriter.cpp
+++ b/clang/lib/APINotes/APINotesWriter.cpp
@@ -649,6 +649,7 @@ namespace {
 unsigned getVariableInfoSize(const VariableInfo &VI) {
   return 2 + getCommonEntityInfoSize(VI) + 2 + VI.getType().size();
 }
+unsigned getParamInfoSize(const ParamInfo &PI);
 
 /// Emit a serialized representation of the variable information.
 void emitVariableInfo(raw_ostream &OS, const VariableInfo &VI) {
@@ -737,6 +738,7 @@ void APINotesWriter::Implementation::writeObjCPropertyBlock(
 namespace {
 unsigned getFunctionInfoSize(const FunctionInfo &);
 void emitFunctionInfo(llvm::raw_ostream &, const FunctionInfo &);
+void emitParamInfo(raw_ostream &OS, const ParamInfo &PI);
 
 /// Used to serialize the on-disk Objective-C method table.
 class ObjCMethodTableInfo
@@ -760,7 +762,10 @@ class ObjCMethodTableInfo
   }
 
   unsigned getUnversionedInfoSize(const ObjCMethodInfo &OMI) {
-    return getFunctionInfoSize(OMI) + 1;
+    auto size = getFunctionInfoSize(OMI) + 1;
+    if (OMI.Self)
+      size += getParamInfoSize(*OMI.Self);
+    return size;
   }
 
   void emitUnversionedInfo(raw_ostream &OS, const ObjCMethodInfo &OMI) {
@@ -768,9 +773,13 @@ class ObjCMethodTableInfo
     llvm::support::endian::Writer writer(OS, llvm::endianness::little);
     flags = (flags << 1) | OMI.DesignatedInit;
     flags = (flags << 1) | OMI.RequiredInit;
+    flags = (flags << 1) | static_cast<bool>(OMI.Self);
     writer.write<uint8_t>(flags);
 
     emitFunctionInfo(OS, OMI);
+
+    if (OMI.Self)
+      emitParamInfo(OS, *OMI.Self);
   }
 };
 
@@ -793,12 +802,22 @@ class CXXMethodTableInfo
     return static_cast<size_t>(key.hashValue());
   }
 
-  unsigned getUnversionedInfoSize(const CXXMethodInfo &OMI) {
-    return getFunctionInfoSize(OMI);
+  unsigned getUnversionedInfoSize(const CXXMethodInfo &MI) {
+    auto size = getFunctionInfoSize(MI) + 1;
+    if (MI.This)
+      size += getParamInfoSize(*MI.This);
+    return size;
   }
 
-  void emitUnversionedInfo(raw_ostream &OS, const CXXMethodInfo &OMI) {
-    emitFunctionInfo(OS, OMI);
+  void emitUnversionedInfo(raw_ostream &OS, const CXXMethodInfo &MI) {
+    uint8_t flags = 0;
+    llvm::support::endian::Writer writer(OS, llvm::endianness::little);
+    flags = (flags << 1) | static_cast<bool>(MI.This);
+    writer.write<uint8_t>(flags);
+
+    emitFunctionInfo(OS, MI);
+    if (MI.This)
+      emitParamInfo(OS, *MI.This);
   }
 };
 } // namespace
diff --git a/clang/lib/APINotes/APINotesYAMLCompiler.cpp b/clang/lib/APINotes/APINotesYAMLCompiler.cpp
index 11578e4d054b7f..3fd9522e001443 100644
--- a/clang/lib/APINotes/APINotesYAMLCompiler.cpp
+++ b/clang/lib/APINotes/APINotesYAMLCompiler.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/YAMLTraits.h"
 #include <optional>
+#include <type_traits>
 #include <vector>
 
 using namespace clang;
@@ -68,7 +69,7 @@ template <> struct ScalarEnumerationTraits<MethodKind> {
 
 namespace {
 struct Param {
-  unsigned Position;
+  int Position;
   std::optional<bool> NoEscape = false;
   std::optional<bool> Lifetimebound = false;
   std::optional<NullabilityKind> Nullability;
@@ -730,7 +731,9 @@ class YAMLConverter {
     }
   }
 
-  void convertParams(const ParamsSeq &Params, FunctionInfo &OutInfo) {
+  std::optional<ParamInfo> convertParams(const ParamsSeq &Params,
+                                         FunctionInfo &OutInfo) {
+    std::optional<ParamInfo> thisOrSelf;
     for (const auto &P : Params) {
       ParamInfo PI;
       if (P.Nullability)
@@ -739,10 +742,16 @@ class YAMLConverter {
       PI.setLifetimebound(P.Lifetimebound);
       PI.setType(std::string(P.Type));
       PI.setRetainCountConvention(P.RetainCountConvention);
-      if (OutInfo.Params.size() <= P.Position)
+      if (static_cast<int>(OutInfo.Params.size()) <= P.Position)
         OutInfo.Params.resize(P.Position + 1);
-      OutInfo.Params[P.Position] |= PI;
+      if (P.Position == -1)
+        thisOrSelf = PI;
+      else if (P.Position >= 0)
+        OutInfo.Params[P.Position] |= PI;
+      else
+        emitError("invalid parameter position " + llvm::itostr(P.Position));
     }
+    return thisOrSelf;
   }
 
   void convertNullability(const NullabilitySeq &Nullability,
@@ -818,7 +827,7 @@ class YAMLConverter {
     MI.ResultType = std::string(M.ResultType);
 
     // Translate parameter information.
-    convertParams(M.Params, MI);
+    MI.Self = convertParams(M.Params, MI);
 
     // Translate nullability info.
     convertNullability(M.Nullability, M.NullabilityOfRet, MI, M.Selector);
@@ -926,11 +935,17 @@ class YAMLConverter {
                          TheNamespace.Items, SwiftVersion);
   }
 
-  void convertFunction(const Function &Function, FunctionInfo &FI) {
+  template <typename FuncOrMethodInfo>
+  void convertFunction(const Function &Function, FuncOrMethodInfo &FI) {
     convertAvailability(Function.Availability, FI, Function.Name);
     FI.setSwiftPrivate(Function.SwiftPrivate);
     FI.SwiftName = std::string(Function.SwiftName);
-    convertParams(Function.Params, FI);
+    if constexpr (std::is_same_v<FuncOrMethodInfo, CXXMethodInfo>) {
+      FI.This = convertParams(Function.Params, FI);
+    } else {
+      if (convertParams(Function.Params, FI))
+        emitError("implicit instance parameter is only permitted on C++ and Objective-C methods");
+    }
     convertNullability(Function.Nullability, Function.NullabilityOfRet, FI,
                        Function.Name);
     FI.ResultType = std::string(Function.ResultType);
diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp
index edb4c19d59745c..5bb58d6c98be95 100644
--- a/clang/lib/Sema/SemaAPINotes.cpp
+++ b/clang/lib/Sema/SemaAPINotes.cpp
@@ -10,10 +10,12 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "TypeLocBuilder.h"
 #include "clang/APINotes/APINotesReader.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Sema/SemaInternal.h"
@@ -567,6 +569,20 @@ static void ProcessAPINotes(Sema &S, FunctionOrMethod AnyFunc,
 static void ProcessAPINotes(Sema &S, CXXMethodDecl *Method,
                             const api_notes::CXXMethodInfo &Info,
                             VersionedInfoMetadata Metadata) {
+  if (Info.This && Info.This->isLifetimebound()) {
+    auto MethodType = Method->getType();
+    auto *attr = ::new (S.Context)
+        LifetimeBoundAttr(S.Context, getPlaceholderAttrInfo());
+    QualType AttributedType =
+        S.Context.getAttributedType(attr, MethodType, MethodType);
+    TypeLocBuilder TLB;
+    TLB.pushFullCopy(Method->getTypeSourceInfo()->getTypeLoc());
+    AttributedTypeLoc TyLoc = TLB.push<AttributedTypeLoc>(AttributedType);
+    TyLoc.setAttr(attr);
+    Method->setType(AttributedType);
+    Method->setTypeSourceInfo(TLB.getTypeSourceInfo(S.Context, AttributedType));
+  }
+
   ProcessAPINotes(S, (FunctionOrMethod)Method, Info, Metadata);
 }
 
diff --git a/clang/test/APINotes/Inputs/Headers/Lifetimebound.apinotes b/clang/test/APINotes/Inputs/Headers/Lifetimebound.apinotes
index d07d87cf02f025..4bd5fbb42bf04c 100644
--- a/clang/test/APINotes/Inputs/Headers/Lifetimebound.apinotes
+++ b/clang/test/APINotes/Inputs/Headers/Lifetimebound.apinotes
@@ -8,6 +8,10 @@ Functions:
 Tags:
 - Name: MyClass
   Methods:
+    - Name: annotateThis
+      Parameters:
+        - Position:      -1
+          Lifetimebound: true
     - Name: methodToAnnotate
       Parameters:
         - Position:      0
diff --git a/clang/test/APINotes/Inputs/Headers/Lifetimebound.h b/clang/test/APINotes/Inputs/Headers/Lifetimebound.h
index 2ec302f7801a77..be0ed14945008f 100644
--- a/clang/test/APINotes/Inputs/Headers/Lifetimebound.h
+++ b/clang/test/APINotes/Inputs/Headers/Lifetimebound.h
@@ -1,6 +1,5 @@
 int *funcToAnnotate(int *p);
 
-// TODO: support annotating ctors and 'this'.
 struct MyClass {
     MyClass(int*);
     int *annotateThis();
diff --git a/clang/test/APINotes/lifetimebound.cpp b/clang/test/APINotes/lifetimebound.cpp
index 3e5ce9df895b70..0e040a6058e439 100644
--- a/clang/test/APINotes/lifetimebound.cpp
+++ b/clang/test/APINotes/lifetimebound.cpp
@@ -1,6 +1,7 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Lifetimebound -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers %s -x c++
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Lifetimebound -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter funcToAnnotate -x c++ | FileCheck --check-prefix=CHECK-PARAM %s
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Lifetimebound -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter annotateThis -x c++ | FileCheck --check-prefix=CHECK-METHOD-THIS %s
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Lifetimebound -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter methodToAnnotate -x c++ | FileCheck --check-prefix=CHECK-METHOD %s
 #include "Lifetimebound.h"
 
@@ -11,3 +12,5 @@
 // CHECK-METHOD: CXXMethodDecl {{.+}} methodToAnnotate 
 // CHECK-METHOD-NEXT: ParmVarDecl {{.+}} p
 // CHECK-METHOD-NEXT: LifetimeBoundAttr
+
+// CHECK-METHOD-THIS: CXXMethodDecl {{.+}} annotateThis 'int *() {{.*}}clang::lifetimebound

@github-actions
Copy link

github-actions bot commented Nov 6, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@Xazax-hun Xazax-hun force-pushed the apinotes-lifetimebound-this branch 2 times, most recently from 80aa4c2 to 3ef43fc Compare November 6, 2024 12:31
This patch makes the position -1 interpreted as the position for 'this'.
Adds some basic infrastructure and support for lifetimebound attribute.
@Xazax-hun Xazax-hun force-pushed the apinotes-lifetimebound-this branch from 3ef43fc to 2c1b035 Compare November 7, 2024 11:47
@Xazax-hun Xazax-hun merged commit 5f4e3a3 into llvm:main Nov 7, 2024
6 of 8 checks passed
Xazax-hun added a commit to swiftlang/llvm-project that referenced this pull request Nov 13, 2024
)

This patch makes the position -1 interpreted as the position for 'this'.
Adds some basic infrastructure and support for lifetimebound attribute.

Cherry-picked from: 5f4e3a3
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
)

This patch makes the position -1 interpreted as the position for 'this'.
Adds some basic infrastructure and support for lifetimebound attribute.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants